Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance Improvement on the Airtable Interface(#1299) #1343

Closed
wants to merge 13 commits into from

Conversation

Abellegese
Copy link
Contributor

@Abellegese Abellegese commented Oct 23, 2024

Thank you for taking your time to contribute to Ersilia, just a few checks before we proceed

  • Have you followed the guidelines in our Contribution Guide
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

Description

A few improvement made on the airtable interface as listed below:

  1. Changing manual yield with more efficient and auto sub-generator creator yield from
    In terms of performance, yield from can be more efficient than manually using yield with a loop. This is because yield from is implemented in C and optimized to handle delegation more efficiently and also has memory improvement. Will report time comparison on the 🐕 Batch: Ersilia CLI Performance Improvement #1299.

  2. Using pyairtable is inevitable so instead of installing it during AirtableInterface call, we can install it as requirements(in pyproject.toml), that reduce the AirtableInterface overhead.

Related to #1299

@miquelduranfrigola
Copy link
Member

Thanks @Abellegese
I am agree with the performance increase and it is a good suggestion. Thanks.
As for the installation of pyairtable, I am less sure. We eventually want to get rid of it. @DhanshreeA can speak more about it. I'll leave it up to both of you to decide how to proceed. At a high level, the way I see it is that AirtableInterface should be a "last resource" option, but definitely not the front-line option.

@Abellegese
Copy link
Contributor Author

Yes @miquelduranfrigola, on the first pull that you merged, I improve 120,000x time the json based model searching functions so this one is just an alternative. Thanks.

@miquelduranfrigola
Copy link
Member

@DhanshreeA let's merge this (or whatever further development you've done) as soon as all is ready.

ersilia/db/hubdata/interfaces.py Outdated Show resolved Hide resolved
@Abellegese Abellegese closed this Oct 26, 2024
@Abellegese Abellegese reopened this Oct 26, 2024
@Abellegese Abellegese closed this Oct 26, 2024
@Abellegese Abellegese deleted the improv-1 branch December 9, 2024 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants